Skip to content

Added and completed all of the test functions for data-and-functions …#3

Open
jikehara wants to merge 1 commit intoAmericaCampaign:masterfrom
jikehara:data-and-functions-1
Open

Added and completed all of the test functions for data-and-functions …#3
jikehara wants to merge 1 commit intoAmericaCampaign:masterfrom
jikehara:data-and-functions-1

Conversation

@jikehara
Copy link

…1 and 2.

Copy link
Contributor

@jcheroske jcheroske left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code is solid. Most of my comments are style-related. I'm looking forward to seeing your next answers.

const getActiveUsers = (data) => {
if (data == null || data.users == null) {
return null
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can dump the else block if you want.

} else {
const activeUsers = []
data.users.forEach((u) => {
if (u.accountActive === true) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've already got a boolean. No need to create another one:

if (u.accountActive) {

return null
} else {
const orders = []
data.orders.forEach((o) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try writing this as a map since you've got the basics down.

if (data == null || data.products == null || id == null) {
return null
} else {
return data.products.find((p) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of find. You can drop the block and use an implicit return if you want. You can also drop the parens around (p) since you only have one param in your lambda.

throw new Error('Missing data or ID')
}
const order = data.orders.find((o) => o.id === id)
if (order === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can totally do the test the way you have it, but you could also just shorten it to:

if (!order) {

return null
}
const productArray = []
order.products.forEach((p) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use map here as well.

@@ -0,0 +1,18 @@
import getProductById from './getProductById'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try importing getProductsForOrder instead and just use that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments